trace: use standard POSIX header for basename() #6
Conversation
|
@dangowrt Would appreciate if you have a chance to look at this. Thanks... |
|
@guidosarducci i searched a bit on the differences and there are a few but considering the following case maybe it's ok... the main difference is that posix variant can modify the passed arg (while GNU one can't) But I assume in the following context it's not a problem since argc is not used in the code after basename is called. |
|
Code was written under the assumption that basename() modifies the argument.
|
Right, I concluded the same thing while reviewing, so thought this should be OK. Thanks for looking. |
guidosarducci
force-pushed
the
master-fix-procd-musl
branch
from
6606631
to
81ae41e
Compare
|
I see that CI checks were just added for procd are are now flagging errors, however the failure looks to be internal and unrelated to this PR. This is a trivial change, has been tested and reviewed, and is needed before a toolchain update to musl 1.2.5. At the moment I have multiple PRs blocked by unrelated CI failures or shortcomings, sitting for several weeks total, with no response from members as to workarounds or fixes. See also openwrt/ubox#4 for example. Any suggestions on how best to proceed @Ansuel @dangowrt @aparcar @ynezz ? |
|
Yes Sorry i will merge your pr to move things up. The error were already
there and some are present but in the context of usage not really security
treat.
Il Sab 30 Mar 2024, 08:53 guidosarducci ***@***.***> ha
scritto:
… I see that CI checks were just added for procd are are now flagging
errors, however the failure looks to be internal and unrelated to this PR.
This is a trivial change, has been tested and reviewed, and is needed
before a toolchain update to musl 1.2.5.
At the moment I have multiple PRs blocked by unrelated CI failures or
shortcomings, sitting for several weeks total, with no response from
members as to workarounds or fixes. See also openwrt/ubox#4
<openwrt/ubox#4> for example.
Any suggestions on how best to proceed @Ansuel <https://github.com/Ansuel>
@dangowrt <https://github.com/dangowrt> @aparcar
<https://github.com/aparcar> @ynezz <https://github.com/ynezz> ?
—
Reply to this email directly, view it on GitHub
<#6 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE2ZMQWTQG2XPM7B2WMRQ5DY2ZVOFAVCNFSM6AAAAABEIRCAU6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXHE2TQMJZGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Indeed, GH CI is really strange, since it seems, that one can't test addition of the CI bits, thus due to that broken pull request was merged, still it should be either reverted or GH actions disabled until this is fixed.
Correct, sorry about that. |
|
Please make your branch writable for maintainers so we can rebase it before merging. Or rebase on top of current procd git HEAD yourself and force-push. |
The musl libc only implements POSIX basename() but provided a GNU header
kludge in <string.h>, which was removed in musl 1.2.5 [1]. Use the standard
<libgen.h> header to avoid compilation errors like:
trace/trace.c: In function 'main':
trace/trace.c:435:64: error: implicit declaration of function 'basename';
did you mean 'rename'? [-Werror=implicit-function-declaration]
435 | if (asprintf(&json, "/tmp/%s.%u.json", basename(*argv), child) < 0)
| ^~~~~~~~
| rename
cc1: all warnings being treated as errors
Link 1: https://git.musl-libc.org/cgit/musl/log/?qt=grep&q=basename
Signed-off-by: Tony Ambardar <[email protected]>
guidosarducci
force-pushed
the
master-fix-procd-musl
branch
from
81ae41e
to
946552a
Compare
@dangowrt Thanks, I've done both yesterday. |
|
Thanks! Rebased on top of master and merged! |
|
Thanks, Robert. |
The musl libc only implements POSIX basename() but provided a GNU header kludge in <string.h>, which is removed in musl 1.2.5 [1]. Use the standard <libgen.h> header to avoid compilation errors like:
Link 1: https://git.musl-libc.org/cgit/musl/log/?qt=grep&q=basename
@blogic This change is related to openwrt/openwrt#14802 and was tested therein.